add SofaRejectedExecutionHandler for user-customized thread pool #1450
Conversation
|
Hi @bohrqiu, welcome to SOFAStack community, Please sign Contributor License Agreement! After you signed CLA, we will automatically sync the status of this pull request in 3 minutes. |
sofastack-cla
bot
added
cla:no
WalkthroughThe changes involve modifications to the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
core/api/src/test/java/com/alipay/sofa/rpc/config/UserThreadPoolManagerTest.java (2)
71-77: LGTM: New test method added correctly.The
testRejectedExecutionHandlermethod effectively verifies that theUserThreadPooluses theSofaRejectedExecutionHandleras intended. This aligns well with the PR objectives.Consider adding a brief comment explaining the purpose of this test for improved readability:
@Test public void testRejectedExecutionHandler(){ + // Verify that UserThreadPool uses SofaRejectedExecutionHandler UserThreadPool userThreadPool = new UserThreadPool(); Executor executorService = userThreadPool.getUserExecutor(); Assert.assertTrue(executorService instanceof ThreadPoolExecutor); Assert.assertTrue(((ThreadPoolExecutor) executorService).getRejectedExecutionHandler() instanceof SofaRejectedExecutionHandler); }
21-21: Summary: Changes effectively implement and test the new feature.The addition of the
testRejectedExecutionHandlermethod, along with the necessary import, successfully verifies the implementation ofSofaRejectedExecutionHandlerfor user-customized thread pools. This change aligns well with the PR objectives and enhances the test coverage for theUserThreadPoolclass.Consider adding similar tests for other types of user-customized thread pools, if applicable, to ensure comprehensive coverage of the new feature across different scenarios.
Also applies to: 71-77
core/api/src/main/java/com/alipay/sofa/rpc/server/UserThreadPool.java (1)
116-116: LGTM! Consider adding import and updating documentation.The addition of
SofaRejectedExecutionHandleraligns well with the PR objectives and enhances the exception handling for user-customized thread pools. This change is consistent with the existing code structure and doesn't affect the current functionality.Consider the following suggestions to improve the code further:
Ensure that the
SofaRejectedExecutionHandlerclass is imported at the top of the file if not already present:import com.alipay.sofa.rpc.server.SofaRejectedExecutionHandler;Update the method's documentation to reflect the new behavior:
/** * Builds and configures a ThreadPoolExecutor with custom settings. * * @return A configured Executor instance with SofaRejectedExecutionHandler */ protected Executor buildExecutor() { // ... existing code ... }These changes will improve code readability and maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- core/api/src/main/java/com/alipay/sofa/rpc/server/UserThreadPool.java (1 hunks)
- core/api/src/test/java/com/alipay/sofa/rpc/config/UserThreadPoolManagerTest.java (2 hunks)
🔇 Additional comments (1)
core/api/src/test/java/com/alipay/sofa/rpc/config/UserThreadPoolManagerTest.java (1)
21-21: LGTM: Import statement added correctly.The import for
SofaRejectedExecutionHandleris necessary for the new test method and is placed appropriately with other imports.
|
|
|
@EvenLjj done |
|
@bohrqiu It's still the same problem. |
|
@EvenLjj oh sorry, i got it . i had format the code and run |
Motivation:
The bizExecutor of all servers (TripleServer, httpServer, BoltServer) are configured with
SofaRejectedExecutionHandlerfor exception handling and log processing， but the user-customized thread pool is missing.Summary by CodeRabbit
New Features
Tests